Windows: Found 0 matching tests #18
Conversation
|
I also have question - what are your prefferences for writing platofrm specific tests eg.: for windows: it is enough? |
|
Now, matching tests are found. At least i get "Found 1 matching test..." But still an error: |
|
@ThomasDeutsch yes i know i reported this error here: facebookarchive/node-haste#12. I will push pull request later to fix this. |
|
@ThomasDeutsch i have pushed PR with fix. Everything should work now |
src/TestRunner.js
Outdated
There was a problem hiding this comment.
Thanks for digging in to this. My guess is there are probably a couple of other places in the code where we need to be better about staying platform-agnostic with regard to paths (better use of path.join and path.sep where appropriate)...but this is a good start.
Can we move this normalization into utils.normalizeConfig() so that anyone who reads the config object gets the benefit of using already-normalized data?
There was a problem hiding this comment.
Yes I want to do this. I thinking about move normalization into new library because i saw there are same problem in others libs eg.: facebookarchive/node-haste#2.
|
The problem with writing platform-specific tests is that they won't fail if someone is developing on another platform and breaks something. It would be much better if the tests mocked out platform-specific APIs and used that to test instead |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@jeffmo fix is ready to merge. Tests are passing under windows an linux |
There was a problem hiding this comment.
Hmm, not all config strings are paths (and thus we shouldn't necessarily normalize them as if they were).
Was there a specific case you were trying to cover with this instead of the places you updated inside normalizeConfig?
There was a problem hiding this comment.
Agree but there is path.resolve used so it will be path after it. Also that line https://github.com/kl3ryk/jest/blob/master/src/lib/utils.js#L50 says that there is <rootDir> inside that string, so we can say this string is a path.
There was a problem hiding this comment.
Ah you're right -- this whole function should only be called on path strings anyway.
Nevermind my previous comments
|
@jeffmo fix is ready to merge. |
There was a problem hiding this comment.
Can you throw this suite into a separate file (say, utils-pathNormalize-test.js) so that it runs in parallel with the other tests?
|
I'm pretty happy with this now -- just want to address the two comments I just now mentioned and I think we're ready to merge |
|
Heya @kl3ryk, thanks a ton for working through this with me. This is going to make a lot of win devs happy :) I've been swamped the last couple of weeks so I haven't had a chance to follow up. If you can split out the |
|
Ahhh sorry i forgot about this :). Tomorrow I will fix all that stuff :) |
Windows: Found 0 matching tests
|
Sorry for the delay but I have a lot of work and will have time for that on weekend. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
4 participants
I tried to run example with sum under windows 7, but i have the following message:
Found 0 matching tests.Problem exists because here: https://github.com/facebook/jest/blob/master/src/TestRunner.js#L122 regexp always return false.
Why? Because on windows directory separator is
\and on some other systems it is/, so some regexps (https://github.com/facebook/jest/blob/master/src/TestRunner.js#L78, https://github.com/facebook/jest/blob/master/bin/jest.js#L198) always gives false.